feat(cells) Make organization-create work in control silo #115238
feat(cells) Make organization-create work in control silo #115238
Conversation
With all the RPC refactoring done, we can make `POST /organizations` work in control silo. Having this endpoint work in both cell & control silo modes is necessary so that we can update internal clients incrementally. These changes also add support for `dataStorageLocation` with locality -> cell resolution which is required for control based provisioning. Refs INFRENG-186
github-actions
Bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| return Response(org_data, status=201) |
There was a problem hiding this comment.
Bug: Due to replica lag, reading a newly created organization for serialization can fail, resulting in an empty response body on a successful 201 creation.
Severity: MEDIUM
Suggested Fix
To ensure the newly created organization is found, perform the read for serialization from the primary database. This can be achieved by forcing the read query within organization_service.serialize_organization to use the primary database, for example by wrapping the call in a block that directs reads to the write replica.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/core/endpoints/organization_index.py#L446-L447
Potential issue: The organization creation endpoint can return an empty response body
with a `201` status code due to replica lag. The organization is provisioned on the
primary database via an RPC. Immediately after, the
`organization_service.serialize_organization` function attempts to fetch this new
organization for the response body. This read operation can be directed to a replica
database which may not have received the new data yet. This causes the serialization
function to return `None`, leading to an empty response body, which violates the API
contract expecting the new organization's data.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b22ff7d. Configure here.
| locality_name = settings.SENTRY_MONOLITH_REGION | ||
|
|
||
| if locality_name is not None: | ||
| attrs["cell_name"] = get_new_org_cell_for_locality(locality_name).name |
There was a problem hiding this comment.
Unhandled CellResolutionError in validate causes 500 error
Medium Severity
get_new_org_cell_for_locality in the validate method can raise CellResolutionError, which is not a DRF ValidationError. DRF's is_valid() only catches ValidationError, so this would bubble up as an unhandled 500 error. The field-level validate_dataStorageLocation properly catches CellResolutionError and converts it, but validate does not — both for user-provided values (where new_org_cell could reference a nonexistent cell) and for the settings.SENTRY_MONOLITH_REGION default (which bypasses validate_dataStorageLocation entirely and is not validated for existence, MULTI_TENANT category, or visibility).
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b22ff7d. Configure here.
1 participant
With all the RPC refactoring done, we can make
POST /organizationswork in control silo. Having this endpoint work in both cell & control silo modes is necessary so that we can update internal clients incrementally.These changes also add support for
dataStorageLocationwith locality -> cell resolution which is required for control based provisioning.Refs INFRENG-186